Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conway #20

Merged
merged 3 commits into from
Dec 21, 2023
Merged

Conway #20

merged 3 commits into from
Dec 21, 2023

Conversation

janmazak
Copy link
Collaborator

@janmazak janmazak commented Nov 2, 2023

No description provided.

@janmazak janmazak force-pushed the conway branch 5 times, most recently from 5eb8036 to 9d8150d Compare November 3, 2023 00:21
src/types.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@gabrielKerekes gabrielKerekes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with NuFi and this test dApp. Seems to work well 👍

I added one minor naming suggestion comment + I pushed a test case with all the certificates which can be generated by the dApp.

src/types.ts Outdated Show resolved Hide resolved
@gabrielKerekes
Copy link
Collaborator

I've stumbled upon an issues I'm not sure how to solve. In NuFi dApp connector we use InteropLib to parse incoming transactions. Then we also try to rebuild the transaction in CSL to make sure we've parsed the tx correctly.

Re-serializing an update_committee governance action doesn't work because the info about unit_interval being a tagged type is lost in the rawGovernanceAction and it's only a simple JS array. And so when we parse the transaction we get the following object:

[ 4, null, [], Map(1) { [Array] => 123 }, [ 1, 4 ] ]

The [ 1, 4 ] array should be Tagged(30, [ 1, 4 ]) and so when I'd like to reserialize the gov action via cbor.encode I basically can't. I would have to manually check and modify the array into a tagged type. I understand it's not feasible to make the parser return the Tagged type but wouldn't it be possible to return the raw cbor for Unparsed or perhaps just for govAction? That would fix the issue since I would just pass the raw bytes into CSL and the interop lib serializer could also be modified to handle this without adding all the gov actions.

This might be a problem for InteropLib as well since you can't parse a transaction and serialize it back if contains a update_committee action.

@janmazak janmazak force-pushed the conway branch 11 times, most recently from 91b32af to 144c43c Compare December 19, 2023 02:00
@janmazak
Copy link
Collaborator Author

This PR should resolve the above-mentioned issue with tag 30.

Copy link
Collaborator

@davidmisiak davidmisiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the TODOs and missing tests, LGTM.

src/utils.ts Outdated Show resolved Hide resolved
src/txParsers.ts Outdated Show resolved Hide resolved
src/txParsers.ts Show resolved Hide resolved
@janmazak janmazak force-pushed the conway branch 2 times, most recently from ddd9057 to dd8f241 Compare December 20, 2023 13:43
@janmazak janmazak marked this pull request as ready for review December 20, 2023 13:44
package.json Outdated Show resolved Hide resolved
@janmazak janmazak merged commit 22e03d0 into develop Dec 21, 2023
4 checks passed
@janmazak janmazak deleted the conway branch December 21, 2023 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants